Skip to content

fix(modal-checkout): open checkout for variation_id URL triggers#163

Merged
rbcorrales merged 5 commits into
mainfrom
fix/modal-checkout-variation-id
Jun 9, 2026
Merged

fix(modal-checkout): open checkout for variation_id URL triggers#163
rbcorrales merged 5 commits into
mainfrom
fix/modal-checkout-variation-id

Conversation

@rbcorrales

Copy link
Copy Markdown
Member

All Submissions:

Changes proposed in this Pull Request:

A modal checkout link of the form ?checkout=1&type=checkout_button&product_id=NNN&variation_id=MMM now opens the checkout modal for the requested variation. Previously these links did nothing for variable subscription and grouped products: no modal appeared, and depending on the product the URL parameters were either stripped from the address bar or left stuck there.

The trigger now resolves the correct target in every configuration:

  • A checkout button locked to the requested variation opens that variation directly.
  • A variation different from the button's locked one is opened through the variation picker, so an arbitrary requested variation is honored.
  • Grouped product children are opened through the picker as well.
  • Variable subscription products now render their picker even when a button is locked to a single variation, so any of their variations can be requested by URL.

When a link requests a variation that cannot be matched to any product or picker, the checkout is no longer silently swapped for a different product. Nothing is submitted, the parameters stay in the URL, and a warning is logged so the broken link can be diagnosed. The URL-driven picker checkout also carries the source button's context (after-success behavior, content gating, popup attribution), matching a manual click. Normal clicks on checkout buttons are unchanged.

Closes NPPM-2872.

How to test the changes in this Pull Request:

These steps assume a site with Reader Activation and modal checkout enabled, WooCommerce and WooCommerce Subscriptions active, a variable subscription product with several variations, a grouped product, and a simple subscription product. Use a page with Checkout Button blocks: one linked to the variable subscription product locked to a specific variation, one linked to the grouped product, and one linked to the simple subscription product. Test logged out (or in a private window) and keep the browser console open throughout.

  1. Visit PAGE?checkout=1&type=checkout_button&product_id=VARIABLE_PARENT_ID (no variation_id). Confirm the modal opens for the button's locked variation and the URL parameters are cleared.
  2. Visit PAGE?checkout=1&type=checkout_button&product_id=VARIABLE_PARENT_ID&variation_id=LOCKED_VARIATION_ID. Confirm the modal opens for that variation.
  3. Visit the same URL but with variation_id set to a different variation of the same product (not the locked one). Confirm the modal opens for that other variation, and that the summary shows the requested variation rather than the locked one.
  4. Visit PAGE?checkout=1&type=checkout_button&product_id=GROUPED_ID&variation_id=CHILD_PRODUCT_ID. Confirm the modal opens for that child product.
  5. Visit a checkout button URL whose variation_id does not belong to the product, for example product_id=SIMPLE_SUBSCRIPTION_ID&variation_id=UNRELATED_ID. Confirm no modal opens, the URL parameters remain in the address bar, and a Newspack modal checkout: no checkout form found... warning is logged.
  6. Throughout steps 1 to 5, confirm there is no Uncaught SyntaxError: "undefined" is not valid JSON in the console.
  7. Confirm normal behavior is unchanged: clicking a locked checkout button goes straight to checkout, and clicking a grouped or chooser button opens the variation picker.
  8. Confirm an after-success setting on the source button (for example a custom button label or redirect) is honored when the checkout is opened through a URL-driven picker variation.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes modal-checkout URL triggers for checkout_button so links that include variation_id reliably open the checkout modal for the exact requested variation (or grouped child), including variable-subscription products, and avoids silent fallbacks when no matching form/picker exists.

Changes:

  • Add DOM helper utilities to resolve the correct checkout form (exact locked button vs. picker selection) and safely parse data-checkout without throwing.
  • Update modal URL-trigger handling to (a) submit only when a matching form is found and (b) keep URL params intact + log a warning when resolution fails.
  • Ensure variable-subscription products render a variation picker modal and fix product-type detection for tiers.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
plugins/newspack-plugin/includes/plugins/woocommerce-subscriptions/class-subscriptions-tiers.php Fixes variable-subscription product type check so tiers logic includes the correct product type.
plugins/newspack-blocks/src/modal-checkout/modal.js Uses new resolver/parsing helpers for URL triggers and prevents param stripping when no matching form is found.
plugins/newspack-blocks/src/modal-checkout/checkout-button-trigger.test.js Adds unit tests covering trigger resolution, picker selection behavior, and safe parsing (no JSON throw).
plugins/newspack-blocks/src/modal-checkout/checkout-button-trigger.js Introduces pure DOM utilities to resolve the correct form for URL-driven checkout-button triggers and copy contextual hidden fields.
plugins/newspack-blocks/src/blocks/checkout-button/view.php Enqueues the modal for the variable product’s parent so the picker is available even when the button is locked to a variation.
plugins/newspack-blocks/includes/class-modal-checkout.php Extends variation selection modal rendering to include variable-subscription products.

Co-authored-by: Codex <noreply@openai.com>
@rbcorrales

Copy link
Copy Markdown
Member Author

Manual QA in an isolated environment

Tested this branch in a fresh isolated environment (WooCommerce, Subscriptions, modal checkout) with a dedicated fixture: a variable subscription product (Monthly $10, Yearly $100) with a checkout button locked to Monthly, a plain grouped product (children $5 and $50), the Newspack donation grouped product, and a simple subscription product.

Results

Scenario Result
product_id=81 (no variation) Opens the locked Monthly variation, params cleared
product_id=81&variation_id=82 (locked) Opens Monthly, params cleared
product_id=81&variation_id=83 (a variation other than the locked one) Opens Yearly via the picker, params cleared. This is the NPPM-2872 case that used to throw
product_id=89&variation_id=88 (plain grouped child) Opens the requested child, params cleared
product_id=47&variation_id=999999 (no match) No modal, params kept, console.warn, no silent product swap
Normal clicks Locked button goes straight to checkout, grouped button opens the picker
After success context The source button after_success_button_label is copied onto the picker submission

No SyntaxError: "undefined" is not valid JSON in any scenario.

One observation

A Newspack auto generated grouped donation product does not open via variation_id, because its picker renders donation frequency forms (hidden product_id inputs) rather than the radio based tier picker the trigger drives, so it falls through to the graceful warn path. The ticket's grouped example (a membership style grouped product) behaves like the plain grouped case above and works. Old behavior here was a SyntaxError crash, so this is still an improvement. Flagging in case deep links to donation tiers by variation_id are a use case worth a follow up.

From a QA standpoint this works as intended for the variable subscription and plain grouped cases.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@rbcorrales rbcorrales marked this pull request as ready for review May 30, 2026 00:28
@rbcorrales rbcorrales requested a review from a team as a code owner May 30, 2026 00:28
@jason10lee jason10lee self-assigned this Jun 8, 2026

@jason10lee jason10lee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved! This works well in testing, is robust against some of the weird edge cases you see in checkout, and neither I nor automated review saw anything that should block merge.

Nit: it's possible someone out there might drop multiple buttons with the same parent ID onto a page (e.g., a subscription product with monthly and annual variations). It looks like findCheckoutButtonForm(root, productId, null) would only match the first matching one from the DOM, so a URL trigger would only pick up that button's context. Feels like an edge case with limited impact (only post-checkout?), but might be worth documenting in a comment if you see the same. Totally non-blocking.

@github-actions github-actions Bot added [Status] Approved Pull request has been approved and removed [Status] Needs Review labels Jun 8, 2026
@jason10lee jason10lee assigned rbcorrales and unassigned jason10lee Jun 8, 2026
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@rbcorrales

Copy link
Copy Markdown
Member Author

Thanks for the review @jason10lee. Confirmed the behavior, and I think it's okay to leave as-is.

With two buttons on the same parent product, findCheckoutButtonForm( root, productId, null ) takes the first in DOM order, so its context wins. But the requested variation always opens correctly. The only thing that follows DOM order is the copied context (after_success_*, gate_post_id, popup attribution), so the impact is post-checkout only.

And the picker path is only reached when no button matches the requested variation, so there's no single "right" button to copy from anyway. A smarter pick wouldn't change your case either, since both buttons are locked.

Went with your suggestion and documented it on the no-variation and picker-copy paths (353adfc).

@rbcorrales rbcorrales enabled auto-merge (squash) June 9, 2026 23:10
@rbcorrales rbcorrales merged commit 9d72c7e into main Jun 9, 2026
10 checks passed
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

Hey @rbcorrales, good job getting this PR merged! 🎉

Now, the needs-changelog label has been added to it.

Please check if this PR needs to be included in the "Upcoming Changes" and "Release Notes" doc. If it doesn't, simply remove the label.

If it does, please add an entry to our shared document, with screenshots and testing instructions if applicable, then remove the label.

Thank you! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-changelog [Status] Approved Pull request has been approved

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants